Conversation
atmiguel
left a comment
There was a problem hiding this comment.
I can't say I agree with putting the database logic into our models. What was the main reason behind that decision?
handlers/tokenutil_test.go
Outdated
| if err != nil { | ||
| panic(err) | ||
| } | ||
| fmt.Println(bob) |
There was a problem hiding this comment.
why are we printing in our tests?
handlers/tokenutil_test.go
Outdated
| } | ||
| } | ||
|
|
||
| func assert(tb testing.TB, condition bool, msg string, v ...interface{}) { |
There was a problem hiding this comment.
I don't know. As you can see the TB is what the package calls it.
handlers/tokenutil_test.go
Outdated
| func assert(tb testing.TB, condition bool, msg string, v ...interface{}) { | ||
| if !condition { | ||
| _, file, line, _ := runtime.Caller(1) | ||
| fmt.Printf("\033[31m%s:%d: "+msg+"\033[39m\n\n", append([]interface{}{filepath.Base(file), line}, v...)...) |
There was a problem hiding this comment.
this fancyness gets the file and line number of the calling method.
handlers/tokenutil_test.go
Outdated
| } | ||
| } | ||
|
|
||
| // equals fails the test if exp is not equal to act. |
There was a problem hiding this comment.
this comment would be unnecessary if the variables just had the full names
There was a problem hiding this comment.
i'll remove the comment
integration_test.go
Outdated
| // Create testing client | ||
| client := &http.Client{} | ||
| { | ||
| // jar, err := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List}) |
models/category.go
Outdated
| "errors" | ||
| ) | ||
|
|
||
| type Category int |
There was a problem hiding this comment.
the filename and everything should probably be noteCategory, instead of just category
| @@ -0,0 +1,2 @@ | |||
| CREATE DATABASE cerealnotes_test; | |||
There was a problem hiding this comment.
why do we need a separate test db?
There was a problem hiding this comment.
This test run against the database, we don't want any bad code to leave extra unexpected data in our database. So for isolation i created a seperate DB.
models/datastore_test.go
Outdated
| } | ||
|
|
||
| func ClearValuesInTable(db *models.DB, table string) error { | ||
| // db.Query() doesn't allow varaibles to replace columns or table names. |
| } | ||
| } | ||
|
|
||
| func ClearValuesInTable(db *models.DB, table string) error { |
There was a problem hiding this comment.
why do we need to clear the values in the tables?
There was a problem hiding this comment.
to make sure every run of the tests runs on the same input.
models/datastore_test.go
Outdated
| } | ||
|
|
||
| // equals fails the test if exp is not equal to act. | ||
| func equals(tb testing.TB, exp, act interface{}) { |
There was a problem hiding this comment.
if we're using these functions everywhere we should put them in a test_util file
There was a problem hiding this comment.
okay we'll just have to be careful about circular dependencies.
No description provided.